[TASK] Add more tests for OutputFormat
#893
Conversation
oliverklee
added this to the
9.0.0: Drop support for PHP < 7.2 and clean up things milestone
JakeQZ
left a comment
JakeQZ
left a comment
There was a problem hiding this comment.
I like the way these tests are narrowing-down expected behaviour.
A couple of minor issues, and some comments beyond the scope of this PR.
tests/Unit/OutputFormatTest.php
Outdated
| /** | ||
| * @test | ||
| */ | ||
| public function createPrettyReturnsNewOutputFormatInstance(): void |
There was a problem hiding this comment.
I think "New" is redundant in the test method name, but it's consistent with createCompactReturnsNewOutputFormatInstance, which I signed off 🤷
| /** | ||
| * @test | ||
| */ | ||
| public function createPrettyCalledTwoTimesReturnsDifferentInstances(): void |
There was a problem hiding this comment.
I'm thinking (and agreeing) "TwoTimes" is better than "Twice" for non-native-English speaking coders.
| { | ||
| $newInstance = OutputFormat::createPretty(); | ||
|
|
||
| self::assertSame([',' => ' '], $newInstance->getSpaceAfterListArgumentSeparators()); |
There was a problem hiding this comment.
Should this just test that the key , has the value {space} - instead of the whole array?
There was a problem hiding this comment.
Or maybe the test method should be renamed to indicate it's checking the whole array by adding On;y at the end of its name.
Co-authored-by: JakeQZ <[email protected]>
|
Updated and repushed. |
No description provided.